Conversation
|
Claude encountered an error —— View job I'll analyze this and get back to you. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request removes the generic HTTP transport handler and its associated configuration, transitioning to provider-specific endpoints for ChirpStack, TTN, and Helium. The review feedback highlights that the sequential numbering in the transport registration comments in serve.go is inconsistent and needs correction. Additionally, the LoRaWAN field in the configuration struct may now be redundant and should be evaluated for removal, and the project documentation in the README requires updates to reflect the removal of the universal /http endpoint in favor of the new provider-specific paths.
| } | ||
|
|
||
| // 6. MQTT Subscriber Transport (non-HTTP) | ||
| // 3. MQTT Subscriber Transport (non-HTTP) |
There was a problem hiding this comment.
The numbering in the comments is inconsistent. This should be 5 to follow the sequence.
| // 3. MQTT Subscriber Transport (non-HTTP) | |
| // 5. MQTT Subscriber Transport (non-HTTP) |
| SMS SMSConfig `mapstructure:"sms"` | ||
| WebSocket WebSocketConfig `mapstructure:"websocket"` | ||
| MQTT MQTTProtocolConfig `mapstructure:"mqtt_protocol"` | ||
| LoRaWAN LoRaWANConfig `mapstructure:"lorawan"` |
There was a problem hiding this comment.
The LoRaWAN field appears to be redundant now that specific LoRaWAN providers (ChirpStack, TTN, and Helium) are defined as top-level fields in ProtocolsConfig. If the map-based configuration is no longer supported, this field should be removed to avoid confusion.
2 participants
What?
Update the url again
Why?
From the request
How?
Testing?
Anything Else?